fix(apps/ensadmin): keep open ENSAdmin GraphiQL docs sidebar#2001
fix(apps/ensadmin): keep open ENSAdmin GraphiQL docs sidebar#2001
Conversation
…pen on the omnigraph page. The editor now memoizes its fetcher, storage, and plugins so 1Hz parent re-renders (driven by the realtime indexing-status projection) no longer trigger schema re-introspection
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
🦋 Changeset detectedLatest commit: bdd730e The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR optimizes the GraphiQL editor component in the ENSAdmin application by memoizing the fetcher, storage, and plugins to prevent unnecessary re-renders and schema re-introspection during frequent parent updates from the realtime indexing-status projection. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 49 minutes and 28 seconds.Comment |
Greptile SummaryThis PR memoizes Confidence Score: 5/5Safe to merge — no P0/P1 issues found; both previously flagged concerns have been addressed. All hooks run unconditionally before the early-return guard, the SSR guard inside the storage memo prevents server-side localStorage access, explorerPlugin() is isolated in its own useMemo([]) for stability, and neither current caller passes a plugins value so EMPTY_PLUGINS fully closes the re-render loop. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Parent as OmnigraphPage (1Hz tick)
participant GE as GraphiQLEditor
participant Memo as useMemo cache
participant GQL as GraphiQL
participant API as /api/omnigraph
Note over Parent,API: Before this PR
Parent->>GE: re-render (url unchanged)
GE->>GQL: new fetcher reference
GQL->>API: schema introspection re-triggered
Note over GQL: Docs sidebar resets
Note over Parent,API: After this PR
Parent->>GE: re-render (url unchanged)
GE->>Memo: useMemo([url]) same fetcher
GE->>Memo: useMemo([url]) same storage
GE->>Memo: useMemo([]) same explorer
GE->>GQL: same fetcher/storage/plugins refs
Note over GQL: No introspection re-run
Reviews (4): Last reviewed commit: "revert: keep explorerPlugin per-instance..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR fixes ENSAdmin’s GraphiQL docs sidebar not staying open on the omnigraph page by stabilizing key GraphiQL prop references across frequent parent re-renders, preventing repeated schema re-introspection.
Changes:
- Memoize the GraphiQL
fetcher,storage, andpluginsto keep stable references across renders. - Hoist the default
pluginsvalue to a module-level constant to avoid busting memoization. - Add a changeset documenting the patch release.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/ensadmin/src/components/graphiql-editor/components.tsx | Memoizes fetcher/storage/plugins to prevent GraphiQL schema re-introspection and UI resets on frequent parent renders. |
| .changeset/large-fans-stop.md | Adds a patch changeset describing the GraphiQL docs sidebar fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/large-fans-stop.md:
- Line 5: The changeset's release-note paragraph is missing a terminal period;
open the changeset's release-note paragraph (the sentence beginning "Fix
ENSAdmin GraphiQL docs sidebar...") and add a period at the end so the paragraph
ends with proper punctuation.
In `@apps/ensadmin/src/components/graphiql-editor/components.tsx`:
- Around line 50-52: The current clear() implementation calls
localStorage.clear() which wipes all origin storage; change clear() in
components.tsx to only remove keys that start with the GraphiQL namespace (the
`ensnode:graphiql:${url}` prefix) by iterating localStorage keys and calling
localStorage.removeItem(key) for matches so only per-URL namespaced entries are
deleted; ensure you reference the same `url`/prefix construction used elsewhere
to compute the namespace.
- Around line 43-64: The storage useMemo factory currently accesses
localStorage.length eagerly (in storage = useMemo(...)) which runs during render
and will throw during SSR before the later typeof window guard; fix by moving
the typeof window check into the useMemo factory (or return a safe fallback) and
make length a lazy getter that reads localStorage.length at access time (and
ensure getItem/setItem/removeItem/clear all reference window.localStorage inside
their functions rather than capturing it at memo time). Update the storage
useMemo so it returns a safe no-op storage when window/localStorage is
unavailable or defines length as a getter to avoid SSR crashes and stale
snapshot values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 21fc13fd-6eab-4768-bad9-45694bf296fc
📒 Files selected for processing (2)
.changeset/large-fans-stop.mdapps/ensadmin/src/components/graphiql-editor/components.tsx
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
- Guard storage useMemo body against SSR (typeof window check) so localStorage access doesn't throw before the early-return; also makes length a getter so it's no longer a stale snapshot. - Restrict storage.clear() to the per-URL namespace prefix so it stops wiping unrelated ENSAdmin localStorage state. - Hoist explorerPlugin() into its own useMemo with [] deps so callers passing an unstable plugins array don't reset the explorer instance. - Trailing period in changeset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review |
…raphiql-introspection-loop
Replace useMemo([]) with a plain module-level const since the plugin takes no inputs and a stable reference is all we need. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The explorer plugin holds editor-scoped state, so a single shared instance would clobber state across multiple GraphiQL editors mounted simultaneously. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review |
…pen on the omnigraph page. The editor now memoizes its fetcher, storage, and plugins so 1Hz parent re-renders (driven by the realtime indexing-status projection) no longer trigger schema re-introspection
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
fetcher,storage, andpluginsinapps/ensadmin/src/components/graphiql-editor/components.tsxso they keep stable references across renders.plugins = []default to a module-level constant so it doesn't bust the memo on every render.if (!url || typeof window === "undefined")early-return below the hooks to keep hook order stable.Why
The omnigraph page's parent ticks once per second via
useNow({ timeToRefresh: 1 })insideuseIndexingStatusWithSwr(driving the realtime indexing-status projection).The
GraphiQLEditorwas rebuilding itsfetcher,storage, and explorer plugin on every render, and GraphiQL re-runs schema introspection whenever thefetcherreference changes. Net result: the docs sidebar would never stay open and the schema was re-fetched every sec.Testing
Notes for Reviewer (Optional)
/api/indexing-statusrefetch and th realtime-projection re-renders are both intentional and unchanged by this PR./api/indexing-statuspolled every 10s, docs sidebar opens and stays open.Pre-Review Checklist (Blocking)